-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for HiFive Unmatched #386
Support for HiFive Unmatched #386
Conversation
Port buildroot & keystone sm to hifive unmatched. Some patches were added from oe-layer, freedom-u-sdk and meta-sifive. This can generate sdcard image for unmatched by single make command.
Buildroot toolchain is configured to use lp64d, but opensbi will use lp64 by default. By adding PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1 to env, opensbi will use compiler matched abi when compiling.
PLATFORM for opensbi and KEYSTONE_PLATFORM don't always coincide. KEYSTONE_PLATFORM should be used when specifying sm special sources.
Changes for testing were mixed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Nanamiiiii!
Thank you for this comprehensive PR -- you've done a great job adding support for this platform to Keystone under the new build system. Attached, please find a few comments/suggestions. We should address these in order to decrease future maintenance costs as Keystone continues to evolve. Of course, all are up for discussion -- please do let me know if you disagree with anything and we can find some common ground :)
Thanks again,
Gregor
overlays/keystone/board/sifive/hifive-unmatched/patches/opensbi/opensbi-change-basename.patch
Outdated
Show resolved
Hide resolved
overlays/keystone/board/sifive/hifive-unmatched/linux-sifive-unmatched-defconfig
Show resolved
Hide resolved
...ys/keystone/board/sifive/hifive-unmatched/patches/opensbi/opensbi-firmware-secure-boot.patch
Outdated
Show resolved
Hide resolved
overlays/keystone/board/sifive/hifive-unmatched/sifive_unmatched_keystone_defconfig
Outdated
Show resolved
Hide resolved
New files related keystone secureboot are located in src/uboot. They are copied into extracted u-boot source before building. patches include only modification in original source.
@dayeol do you mind giving this a brief glance before approval? I think this looks pretty great, but just want to run it by you before merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! This aligns well with the goal of the project to make it portable across many RISC-V platforms. Your contribution will really help others to use this project for theirs with more available unmatched boards.
I have a question and a minor concern regarding the added license, which we could resolve together. I'll get back really soon :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe GPL2 is a copyleft license and inclusion of it would affect some requirements. I will consult this with CCC. Meanwhile, is there a way this license could be completely removed from the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is attached to the systemd unit and udev rule for LED control of the board, and may be removed if they are not needed.
It is possible to download the sources directly from freedom-u-sdk at build time without including them in Keystone's repository, but I don't know if that would affect the license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this code is completely separate from our repo, I would like to keep it excluded from our repo as it might be confusing & complicated to maintain.
Could you please make it download the sources directly from freedom-u-sdk?
Other than that, I'm good with the PR. Thanks in advance!
overlays/keystone/board/sifive/hifive-unmatched/src/uboot/keystone/Makefile
Outdated
Show resolved
Hide resolved
I found the cause of this issue. While first board initialization process of u-boot, it use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please exclude GPL-licensed code from the PR and have it download the code at build time if necessary.
White early initialization, U-Boot stretch stack to lower addr space from its load addr `0x80200000`. So the stack will touch pmp protected space and U-Boot hangs due to access violation. To avoid this, U-Boot load addr is changed to upper addr, `0x80210000`.
Some files imported from freedom-u-sdk are GPL licensed. So they're removed from keystone repo and directly downloaded at build-time.
This PR adds board support for HiFive Unmatched. (#384)
You can generate SD card image for unmatched using buildroot by single
make
command.It will not be available for production, but it will allow for easy testing.
I checked that almost all example runs correctly, but only
attestor.ke
throw runtime error bacause the package does not include firmware image (fw_jump.elf
) correctly. Same thing is happening with qemu target.New items/Changes
overlays/keystone/configs
overlays/keystone/board
freedom-u-sdk
&meta-sifive
(2023.08) are includedsm/plat
Known issue
Porting issue mentioned here is still remaining.
With default
SMM_SIZE
(0x200000
), machine hangs when switching to S-Mode entering u-boot proper (when functionsbi_hart_switch_mode
is called).In this PR, change
SMM_SIZE
to0x80000
same as sbi domain region by passing compile flag for unmatched target. This is temporary fix.Remarks
FU740 has waymasks same as FU540. Waymasking implemented for FU540 can be ported to FU740, I think.